Update to 1.7.0 alpha.2#881
Conversation
xylar
added
documentation
|
This needs: |
xylar
force-pushed
the
update-to-1.7.0-alpha.2
branch
from
62fd06a to
fe92eb2
Compare
xylar
force-pushed
the
update-to-1.7.0-alpha.2
branch
from
c27fa8a to
c30a74c
Compare
TestingI ran both the |
|
@trhille and @matthewhoffman, I will need your help to make sure I didn't break anything for you 2. Can you run tests that go beyond |
| write_netcdf(dsMesh, 'grid_converted.nc') | ||
| levels = section.get('levels') | ||
| args = ['create_landice_grid_from_generic_MPAS_grid.py', | ||
| args = ['create_landice_grid_from_generic_mpas_grid', |
There was a problem hiding this comment.
There was a problem hiding this comment.
I would have to remind myself of the details. You should not try to call the entry-point functions because the command-line arguments are still expected for those and they won't work as expected. But in many cases there are other functions with "proper" argument you could call instead and that might be preferable.
In any case, yes, I tried to change things minimally.
There was a problem hiding this comment.
Looking at this further:
https://github.com/MPAS-Dev/MPAS-Tools/blob/701c8d97dfa733ec7128f32aff5ddd4f48a14e2c/conda_package/mpas_tools/landice/create.py#L10
The function create_from_generic_mpas_grid() as written is not really callable because it assumes that OptionParser.parse_args() will return the expected command-line arguments (which compass obviously wouldn't do). Elsewhere in mpas_tools, we have a function with proper arguments that can be called directly and then we have entry point function that do arg parsing and then call the other function. It would be great to move in that direction but I didn't try to do that for you all because it was one step too far for me.
There was a problem hiding this comment.
@xylar , got it - that makes sense. I'm happy with changing things in this way for now (here and the companion MPAS-Tools PR).
trhille
left a comment
trhille
left a comment
There was a problem hiding this comment.
Approved based on the testing described here: MPAS-Dev/MPAS-Tools#596
xylar
force-pushed
the
update-to-1.7.0-alpha.2
branch
4 times, most recently
from
2ff5ff2 to
fa44af4
Compare
The NetCDF4 helper function from pyremap is no longer available, and pyremap has switched to this xarray syntax.
We want to make sure they're removed, but it doesn't matter if they weren't installed in the first place.
The environment name needs to be set. The test has also been given a better name.
xylar
force-pushed
the
update-to-1.7.0-alpha.2
branch
from
fa44af4 to
044ace2
Compare
|
@matthewhoffman and @trhille, the versions of pyremap and mpas_tools that this update depends on have been released. There is a non-zero chance that I've missed a few updates needed for either of these. We can either do more testing now, before this goes in or we can go ahead and merge this, and fix anything that remains in follow-up PRs. Basically, it's a question of how much you want to suffer now vs. potentially suffer later... |
|
I'll certainly re-test the |
Further testing
|
matthewhoffman
left a comment
matthewhoffman
left a comment
There was a problem hiding this comment.
Approving based on inspection and @trhille's testing
|
Thanks @trhille and @matthewhoffman!! |
3 participants
This brings in:
As a result of the
mpas_toolsupdate, many command-line tools used bylandicehave been renamed. The relevant documentation has also been updated.Checklist
Testingin this PR) any testing that was used to verify the changes